Skip to content

fix(ai): #850 follow-ups — target-dir event, one Ollama reader for any host, no gemini fallback - #852

Merged
fas89 merged 4 commits into
mainfrom
fix/copilot-850-followups
Nov 12, 2026
Merged

fas89 merged 4 commits into
mainfrom
fix/copilot-850-followups

Conversation

@fas89

@fas89 fas89 commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator

Description

These are follow-ups to #850, found by the forge_docs review of #850 (Agenticstiger/forge_docs#204) and by two reviews of this diff. There is one commit per fix.

  1. fix(forge): --agent reports a contract written to a --target-dir outside the cwd.
    • The receipt and the forge.contract_written event came from a snapshot of the cwd only, two levels deep. A target outside it, or deeper, got neither.
    • The stated target is now snapshotted too, with ~ expanded as the writer expands it. A file outside the cwd is reported by its absolute path.
  2. fix(ai): one Ollama address for every read, any host, from the stated endpoint.
    • OLLAMA_HOST was read four ways. The chat call dialled any host. Detection, the default-model pick and fluid ai setup ignored a non-local host and asked localhost, so the model came from one daemon and the call went to another.
    • fluid_build/llm/ollama_host.py is now the single reader. It follows ollama-python's _parse_host convention (MIT).
    • A stated endpoint now drives the model pick, the model preflight and the fluid ai test model check. A --tiered stage keeps it too.
    • Credentials in the address: they are sent as basic auth; fluid doctor and fluid ai setup show them masked; ai_config.json doesn't store them unless FLUID_ALLOW_PLAINTEXT_AI_SECRETS is set; and the environment's OLLAMA_HOST wins over the saved host.
  3. fix(ai): stop with copilot_no_llm_provider instead of guessing gemini.
    • With nothing selecting a provider, forge used gemini. It sent FLUID_LLM_API_KEY to Google whatever key it was, and with two provider keys set it asked for a gemini key.
    • It now stops with the reason, in the same words fluid doctor shows.

Two owner decisions (2026-11-12) set the behaviour:

  • OLLAMA_HOST may name any machine, as ollama-python allows. It is already a protected name that a project .env cannot set.
  • There is no silent provider fallback.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would break existing functionality)
    • A run whose only setting is FLUID_LLM_API_KEY now stops instead of using gemini.
    • fluid ai test --endpoint and fluid ai setup accept a non-local Ollama.
    • LlmProvider.list_available_models takes an endpoint keyword. It is passed only when an endpoint is stated, so a two-argument plugin keeps working until then.
  • Documentation update
  • Refactor / code cleanup
  • New provider or provider enhancement

Documentation

Checklist

  • I have read the Contributing Guide
  • My code follows the project's coding standards
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass (pytest)
  • I have run ruff check and black with no errors
  • New maintained Python files include license headers (python scripts/add_license_headers.py; examples/** exempt)

Testing

Live A/B runs through the fluid CLI, against main at 28a1f2e:

  • The runs were isolated from the machine's own setup: temp HOME, null keyring backend, and HTTP_PROXY pointed at a dead port to hide the local Ollama. A stub Ollama daemon served the remote-host checks.
  • fluid forge --agent --llm-provider ollama --llm-model qwen3-coder:30b -d <outside cwd>
    • main: no forge.contract_written and no forge-receipt.json.
    • This branch: forge.contract_written with the absolute path, action: created, and the receipt written.
  • fluid ai test --provider ollama --endpoint http://127.0.0.1:11434, with OLLAMA_HOST set to a stub that lacks the model
    • main: exit 4, "model ... was not returned by the provider".
    • This branch: model available, FLUID_OK, exit 0.
  • fluid forge with OLLAMA_HOST on a LAN-IP stub and no --llm-model
    • main: "OLLAMA_HOST points to a non-localhost address (...), ignoring". It picked gemma4:31b from the local daemon while the chat went to the stub.
    • This branch: no warning, and the model and the chat both come from the stub.
  • fluid forge with OPENAI_API_KEY and ANTHROPIC_API_KEY set and nothing else
    • main: "No API key was configured for the gemini copilot adapter".
    • This branch: names both keys and asks for --llm-provider. fluid doctor shows the same message.

Tests:

  • New: tests/test_ollama_host.py, tests/test_llm_no_provider.py, and 4 tests in tests/cli/test_forge_agent_mode.py. A verifier ran them against main: they fail there and pass here.
  • The 63 related test files, tests/copilot and the startup budget give 3442 passed. Commits 1 and 2 are each green on their own, at 3380 and 3422 passed.
  • Full suite locally: 34485 passed. The 9 failures and 2 collection errors fail identically on main in the shared venv (dlt version drift, missing bashlex, the lazy-core and duckdb-sandbox baseline), or are load-sensitive and pass when run alone.
  • fluid --help startup budget: 301 modules, the same as main.

Reviews:

  • A Sonnet verifier ran twice.
  • An Opus security review found no new egress path and no non-operator control of the Ollama address. Its 3 credential findings (plaintext save, terminal print, username-only token) are fixed here.

Command Center is unaffected. It always passes --llm-provider with a matching key, never sets OLLAMA_HOST, and has no LlmProvider subclass.

Not in this PR

  • FLUID_LLM_API_KEY can still reach a provider forge only inferred, such as an ambient Ollama or a keyring-inferred provider. This is older than this change, and its rule needs an owner decision.
  • Non-interactive runs still don't read the saved fluid ai setup choice. The new message names the saved provider and the flag to pass.

…ide the cwd

The receipt and the forge.contract_written event come from a before/after
snapshot that scanned only the current directory, two levels deep. A
--target-dir outside it, or deeper, got neither: an AI-mode run wrote the
contract and the JSONL stream never said so. The stated target is now
snapshotted too, with ~ expanded as the writer expands it, and a file outside
the current directory is reported by its absolute path.

Found by the forge_docs review of #850.
… endpoint

OLLAMA_HOST was read four ways. The chat call dialled any host, but detection,
forge's default-model pick and fluid ai setup ignored a non-local host with a
warning and asked localhost, so the model came from one daemon and the call
went to another. fluid ai test --endpoint refused a non-local Ollama outright.
Owner decision: any host, as in ollama-python. OLLAMA_HOST is already a
protected name that a project .env cannot set.

- fluid_build/llm/ollama_host.py is the one reader, following ollama-python's
  _parse_host convention (MIT). It keeps user:password@, sent as basic auth,
  and reads the scheme case-insensitively.
- A stated endpoint (--llm-endpoint, FLUID_LLM_ENDPOINT, ai test --endpoint)
  now drives the default-model pick, the model preflight and the ai test model
  check, which asked OLLAMA_HOST or localhost. A --tiered stage keeps a stated
  endpoint, as LlmConfig.for_routing already did.
- LlmProvider.list_available_models takes an endpoint keyword, passed only when
  one is stated. The Ollama model list moved from urllib to httpx, so basic
  auth works and no redirect is followed; a non-http(s) URL is refused.
- Credentials: fluid doctor and fluid ai setup show the address masked
  (providers.redact_endpoint), ai_config.json stores it without them unless
  FLUID_ALLOW_PLAINTEXT_AI_SECRETS is set, and the environment's OLLAMA_HOST
  wins over the saved host.
With nothing selecting a provider, forge used gemini. It sent
FLUID_LLM_API_KEY to Google whatever key it was, and with two provider keys
set it asked for a gemini key, while fluid doctor said "Not selected". Owner
decision: no silent fallback.

resolve_llm_config now raises copilot_no_llm_provider before any provider is
built, with no_provider_message(env), shared with check_llm_readiness so forge
and doctor say the same thing: nothing configured, several keys set, a key that
names no provider, or a saved fluid ai setup choice the command does not read.
The error catalog carries the suggestions.
@github-actions github-actions Bot added security Security-related changes or reports cli Changes to the CLI surface or implementation tests Test coverage or test infrastructure changes labels Nov 12, 2026
Coverage & Audit (Python 3.14) failed on test_inline_setup_keeps_the_credentials_in_ollama_host
with "'NoneType' object has no attribute 'provider'": run_ai_setup_inline returns None when an
earlier test on the same worker left the module-level _ai_setup_skipped set, and random test
order put one first. The test now resets the flag, as the other inline-setup tests do, points
HOME at its tmp dir and pins the litellm check, so it reads nothing of the machine it runs on.
@fas89
fas89 merged commit 3900c45 into main Nov 12, 2026
38 checks passed
fas89 added a commit that referenced this pull request Nov 12, 2026
CHANGELOG.md conflicted: both sides added to [Unreleased]. Both entry sets
are kept, with #852's three Changed entries moved into the branch's single
Changed section. _error_catalog.py merged cleanly; error-codes.md is
regenerated for #852's new catalog entry.
fas89 added a commit that referenced this pull request Nov 12, 2026
Moves [Unreleased] under [0.29.0] — 2026-11-12, leaving an empty
[Unreleased] above it. release.yml extracts this section as the GitHub
Release body: 22 entries under Added, Changed, Fixed and Security.

Two corrections ride along so the release notes are accurate:
- #854 merged without a CHANGELOG change. Its three entries are added:
  runs without a terminal use the saved fluid ai setup choice, --agent
  stdout carries only the JSON-Lines events, and fluid mission run no
  longer picks a coding agent it cannot use.
- #852's no-provider entry quoted a "does not read the saved choice"
  variant that #854 replaced; it now names the saved coding agent the run
  passes over.
fas89 added a commit that referenced this pull request Nov 12, 2026
…litellm banner, one contract event (#858)

* fix(ai): litellm prints nothing to stdout, so `ai test --json` is JSON

On a failed call litellm printed its "Give Feedback / Get Help" banner to
stdout, ahead of `fluid ai test --json`'s report. quiet_litellm() sets
litellm's own switch, suppress_debug_info (the one its Router and proxy set,
BerriAI/litellm#5942), wherever forge loads litellm: _get_litellm,
ai_setup._validate_api_key and the agent loop's tool call. The failure still
reaches forge as the exception, and forge reports it as before.

The tests use the installed litellm: mock_response=Exception() fails inside
litellm's error mapping, where the banner is printed, with no network call.

* fix(forge): `--agent` reports a failure as a forge.error event, not a traceback

A typed AI error such as copilot_no_llm_provider printed a full traceback on
stderr: _create_project_minimal logged it with logger.exception, which the
CLI's console handler prints at the default INFO level, in every mode. The
--agent stream said only forge.done with exit_code 1.

log_forge_failure (cli/_forge_mode_errors.py) logs a FLUID error once at
ERROR without the traceback, kept off the console when the caller prints the
message itself, and the traceback at DEBUG; a crash keeps logger.exception.
It notes the failure in a ContextVar, and forge.run sends it as one
forge.error event before forge.done: code, message, suggestions and docs_url,
redacted; an unexpected exception by its class only, since MCP forge_run
hands the events to the IDE's model. Template mode, the agent loop, the
minimal path, the mode boundary and _run_main use it; _run_main now prints a
FluidUserError's fix. A failed --seed-from notes the console's words, not
its exception's text. MCP forge_run writes each forge.error to the server's
log, since it discards forge's console output.

* fix(forge): `--agent --blank` reports its contract once

Blank mode emitted its own forge.contract_written and the receipt emitted
another for the same file, first by its absolute path and then by its path
from the current directory. Since #852 the receipt scans every --target-dir,
so blank mode's own event goes, as in every other mode.

* fix(forge): a failed `--seed-from` names what it accepts, not a missing --verbose

The console line ended "(run with --verbose for details)", and the new forge_seed_failed event repeated it as a suggestion, but forge has no --verbose flag (exit 2). Both now say what load_seed accepts: a *.odps.yaml or *.odcs.yaml file, or a directory of them.

* fix(mcp): log a forge_run failure on one line

_log_forge_errors wrote the forge.error message with %s, so a newline in it could add a made-up line to the server log. %r keeps it on one line (security re-check, LOW).

* fix(forge): `--agent` help lists the events the stream sends

The help named forge.start, forge.done, forge.contract_written and forge.cost. forge.cost is never sent, and forge.error (new), forge.file_written and forge.plan were missing (delivery review, P2).

* fix(ai): the agent loop's summarizer quiets litellm too

The summarizer imports litellm itself and calls litellm.completion, so it gets quiet_litellm like the other three callers (defect scan, Low). TestLogForgeFailure also drops the note it leaves, with the traceback frames it holds, after each test.

* fix(forge): the `--seed-from` hint names every input load_seed takes

It named only *.odps.yaml / *.odcs.yaml files and directories; load_seed also takes .yml / .json names and any file with kind: DataProduct or DataContract (fix re-check, P3).

* test: address the code-quality review on #858

The failure helpers catch Exception, not BaseException; the boundary test raises through a helper call inside the with, so the line after it is reachable; tests/test_litellm_banner.py imports fluid_build.cli.ai_setup one way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Changes to the CLI surface or implementation security Security-related changes or reports tests Test coverage or test infrastructure changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant